Skip to content

build: collect per-test timing data via --report-log#38287

Draft
feanil wants to merge 7 commits intomasterfrom
feanil/collect-test-timings
Draft

build: collect per-test timing data via --report-log#38287
feanil wants to merge 7 commits intomasterfrom
feanil/collect-test-timings

Conversation

@feanil
Copy link
Copy Markdown
Contributor

@feanil feanil commented Apr 4, 2026

Summary

  • Adds --report-log to the pytest invocation in unit-tests.yml so each shard emits a JSONL file with per-test durations
  • Uploads the report file as a CI artifact per shard
  • This is a temporary data-collection branch — timing data from the CI run will be used to drive optimal rebalancing of test shards

@feanil feanil force-pushed the feanil/collect-test-timings branch from 4384183 to afef03c Compare April 4, 2026 14:12
Run pytest with extra reporting enabled to generate files with per-test
durations. The file is uploaded as a CI artifact so timing data can be
downloaded and used to drive optimal shard rebalancing.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@feanil feanil force-pushed the feanil/collect-test-timings branch from afef03c to c17633d Compare April 4, 2026 18:45
feanil and others added 2 commits April 4, 2026 23:36
Redistribute test paths across 9 shards (down from 16) using a greedy
bin-packing optimiser driven by real per-test timing data from
pytest-reportlog. Predicted critical path: ~18.7m (down from ~29m).

Key changes:
- Rename shard groups to reflect semantic meaning: lms-*, shared-with-lms-*,
  shared-with-cms-*, cms-* (openedx/common/xmodule paths explicitly separated
  from lms-only and cms-only paths)
- Split lms/djangoapps/discussion/ into its 4 subdirectories so the heavy
  rest_api/ shard (15.7m) can be distributed across bins independently
- Remove outdated comment referencing unit-tests-gh-hosted.yml

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ubclasses

Three test classes in the certificates app were calling CourseFactory() in
setUp() despite extending SharedModuleStoreTestCase. Unlike ModuleStoreTestCase,
SharedModuleStoreTestCase shares a single modulestore across all tests in the
class and only closes MongoDB connections at tearDownClass. Calling
CourseFactory() in setUp() created a new MongoDB course (and opened connections)
for every test method without releasing them, causing connection accumulation
across the full test run.

Affected classes:
- CertificateFiltersTest (test_filters.py)
- CertificateInvalidationTest (test_models.py)
- CertificateAllowlistTest (test_models.py)

In each case the course is only read by test methods (test data such as users,
enrollments and certificates is written via Django ORM and rolled back between
tests), so sharing a single course across the class is correct.

See: https://github.com/openedx/openedx-platform/blob/master/xmodule/modulestore/tests/django_utils.py

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@feanil
Copy link
Copy Markdown
Contributor Author

feanil commented Apr 6, 2026

The fix needs to be made upstream: openedx/openedx-events#559 waiting for that to be merged and released before coming back to this PR.

feanil and others added 4 commits April 8, 2026 11:57
…vents.testing

openedx_events/tests/utils.py was moved to openedx_events/testing.py in
openedx/openedx-events#559 so the test utilities are included in the
installed package (setup.py excludes the tests/ subpackage from the wheel).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When OpenEdxEventsTestMixin was listed after a TestCase subclass (e.g.
Foo(SharedModuleStoreTestCase, OpenEdxEventsTestMixin)), it landed after
unittest.case.TestCase in the MRO. Since unittest.case.TestCase.setUpClass
and tearDownClass do not call super(), the mixin's lifecycle methods never
ran. The workaround was to manually call cls.start_events_isolation() in
each class's setUpClass, but there was no corresponding tearDownClass to
restore event state, causing events disabled by one test class to leak into
subsequent classes in the same process.

Fix by placing OpenEdxEventsTestMixin first in the base class list so it
appears before unittest.case.TestCase in the MRO. This lets setUpClass and
tearDownClass run automatically through the cooperative super() chain,
removing the need for manual start_events_isolation() calls.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Pins openedx-events to the openedx/openedx-events#559 branch which
contains two fixes not yet in a released version:

- tearDownClass added to OpenEdxEventsTestMixin to restore event state
- Test utilities moved to openedx_events.testing so they are included
  in the installed package (setup.py excludes tests/ from the wheel)

Revert to unpinned once openedx/openedx-events#559 is merged and
released as 11.1.1.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This mixin is already included via one of the other mixins on this test
class so including it again was messing with the MRO for the test
classes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant